fix: guard ThemeProvider localStorage access against SecurityError#1178
Merged
sw-factory-automations merged 2 commits intoMay 24, 2026
Merged
Conversation
Wrap localStorage.getItem and localStorage.setItem in try-catch in theme.tsx to prevent crashes when storage access is denied (restricted browsers, privacy settings, embedded contexts). Falls back to dark theme on read, silently ignores on write. The inline theme script in layout.tsx already had this protection, but ThemeProvider did not — causing a SecurityError that broke the entire React render tree for affected users. Co-authored-by: Ona <no-reply@ona.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Collaborator
Author
|
Review complete — no issues found. ✅ The fix is minimal, correct, and well-tested:
Merge blocked by pending Vercel status check — the Vercel preview deploy has been pending since 00:11 UTC (30+ min). All CI checks (lint, typecheck, tests, e2e, visual, bundle) have passed. Will merge once the Vercel check resolves. |
Co-authored-by: Ona <no-reply@ona.com>
Collaborator
Author
|
✅ UI verification skipped — no UI changes detected. Changed files ( |
Collaborator
Author
|
✅ Post-merge verification passed. E2E tests (against live site):
Ad-hoc smoke tests:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wraps
localStoragecalls inThemeProviderwith try-catch to prevent crashes when storage access is denied.Closes #1177
Sentry Issue
MEMO-2H —
SecurityError: Failed to read the 'localStorage' property from 'Window': Access is denied for this document.— 4 events, 2 users, Chrome Mobile 149 on Android.Root Cause
ThemeProviderinsrc/lib/theme.tsxcallslocalStorage.getItem()(line 31) andlocalStorage.setItem()(line 41) without try-catch. Browsers with restricted storage access (privacy settings, embedded contexts, enterprise policies) throw aSecurityError(DOMException), which propagates through React's render tree and breaks the entire app.The inline theme script in
layout.tsxalready had identical try-catch protection. Theuse-persisted-expanded.tshook also wraps localStorage in try-catch. Onlytheme.tsxwas unprotected.Changes
src/lib/theme.tsx— WraplocalStorage.getIteminuseStateinitializer andlocalStorage.setIteminsetPreferencewith try-catch. Falls back to"dark"on read, silently ignores on write.src/lib/theme.test.tsx— New regression test covering: default behavior, stored preference reading, invalid value handling, preference persistence, and SecurityError handling for both getItem and setItem.src/lib/sentry/sentry.test.ts— Addtheme.tsxto the bare-catch permanent allowlist (same pattern asuse-persisted-expanded.ts)..agents/conventions.md— Document localStorage safety requirement in the Theme section.Verification
pnpm lint— 0 errorspnpm typecheck— cleanpnpm test— 1981/1981 tests pass (including 6 new theme tests)